gh-102828: add onexc arg to shutil.rmtree. Deprecate onerror.#102829
gh-102828: add onexc arg to shutil.rmtree. Deprecate onerror.#102829iritkatriel merged 4 commits intopython:mainfrom
Conversation
|
I like the idea in principle, but existing code using I'm not sure what's the current policy re. deprecations, but I suppose there can be a transitional time (2 releases? 3?) during which using try:
...
except OSError as err:
if onerror is not None:
warnings.warn("'onerror' argument is deprecated and will be removed in XXX. Use 'onexc' argument instead.",
DeprecationWarning)
onerror(..., ..., sys.exc_info())
if onexc is not None:
onerror(..., ..., err) |
It will still work, because onerror gets wrapped by onexc in Lib/shutil.py:708-714. Note that I didn't remove any tests, so all previous functionality is still there. |
|
Oh you're right sorry. I misread your patch. |
|
When you're done making the requested changes, leave the comment: |
|
When you're done making the requested changes, leave the comment: I have made the requested changes; please review again. |
|
Thanks for making the requested changes! @giampaolo: please review the changes made to this pull request. |
|
|
|
@iritkatriel looks like this broke some build bots. |
|
I see what I did - I copied part of this test from another test, but I didn't copy the skip instructions that make it not run where it doesn't work. Will fix. |
|
Ignore my experiment with the 1st buildbot report. |
|
Hey, why does If it accepted only one argument, then:
The only thing lost would be the |
Because that's what onerror did. This PR was just about replacing exc_info by exc. Feel free to suggest additional changes in a separate issue. |
|
Righto, thanks. I've logged #103218. |
onexc expects an exception instead of exc_info. This is part of the larger effort to move on from exc_info.
Fixes #102828.